Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: respect inspect() depth #27983

Merged
merged 1 commit into from
Jun 1, 2019
Merged

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented May 30, 2019

This commit causes Http2Stream and Http2Session to account for inspect() depth.

Fixes: #27976

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@renanbastos93
Copy link

LGTM, but I have a question.

I did read this code and saw https://github.com/nodejs/node/pull/27983/files#diff-696b2cc418addca5f3fe5020058f8b15R1071 but I don't understand this syntax [somethin](){}.
Can you explain to me?

@cjihrig
Copy link
Contributor Author

cjihrig commented May 30, 2019

Can you explain to me?

That syntax is called a computed property/method name. The linked MDN page will do a better job explaining it than I probably will in a GitHub comment.

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, didn’t think of something as nice as just returning this :)

@nodejs-github-bot
Copy link
Collaborator

@BridgeAR BridgeAR added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. http2 Issues or PRs related to the http2 subsystem. labels May 31, 2019
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 1, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/23530/

EDIT(cjihrig): CI was yellow.

This commit causes Http2Stream and Http2Session to account
for inspect() depth.

PR-URL: nodejs#27983
Fixes: nodejs#27976
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@cjihrig cjihrig merged commit f86100c into nodejs:master Jun 1, 2019
@cjihrig cjihrig deleted the custom-inspect branch June 1, 2019 19:16
targos pushed a commit that referenced this pull request Jun 3, 2019
This commit causes Http2Stream and Http2Session to account
for inspect() depth.

PR-URL: #27983
Fixes: #27976
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@targos targos mentioned this pull request Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

http2: custom inspect functions ignore depth
8 participants